Skip to content

TST: Extend datetime64 arith tests to array classes, fix several broken cases #23771

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Nov 19, 2018

Conversation

jbrockmendel
Copy link
Member

Some of the DateOffset arithmetic didn't work with the EA classes; this fixed that. Other pieces of the DateOffset arithmetic will have to wait until TimedeltaArray gets a few more methods.

@pep8speaks
Copy link

pep8speaks commented Nov 18, 2018

Hello @jbrockmendel! Thanks for updating the PR.

Comment last updated on November 18, 2018 at 21:11 Hours UTC

if self.freq is not None:
return type(self)(-self._data, freq=-self.freq)
return type(self)(-self._data)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be made unnecessary by #23642.

# RelativeDelta DateOffsets

# -------------------------------------------------------------
# Non-Tick, Non-RelativeDelta DateOffsets
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of the tests currently at the bottom of this file will end up in these two sections. For now leaving them in place to make the diff clearer.

@jreback jreback added Testing pandas testing functions or related to the test suite ExtensionArray Extending pandas with custom dtypes or arrays. labels Nov 18, 2018
@@ -791,6 +795,10 @@ def __sub__(self, other):
else: # pragma: no cover
return NotImplemented

if is_timedelta64_dtype(result) and isinstance(result, np.ndarray):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

worth comments here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really. It would just say # wrap result in TimedeltaArray which is pretty redundant given the one-liner here

pd.offsets.MonthEnd()])
exp = klass([Timestamp('2001-1-1'), Timestamp('2000-2-29')])
other = tm.box_expected(other, box_with_array)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this file getting kind of hefty. ok with making a subdir if it helps, e.g.

pandas/tests/arithmetic/datetime64/test_offsets....etc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm hopeful we'll be able to de-duplicate a bunch of these (many moved from series/frame/index tests) and slim it down. If not then subdirs may be the way to go.

@jreback
Copy link
Contributor

jreback commented Nov 18, 2018

looks good. ping on green.

@jreback jreback added this to the 0.24.0 milestone Nov 18, 2018
@jbrockmendel
Copy link
Member Author

looks good. ping on green.

Travis fail is the test_apply segfault. gentle ping

@jreback jreback merged commit 0ab8eb2 into pandas-dev:master Nov 19, 2018
@jreback
Copy link
Contributor

jreback commented Nov 19, 2018

thanks

@jbrockmendel jbrockmendel deleted the dtests branch November 19, 2018 02:09
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ExtensionArray Extending pandas with custom dtypes or arrays. Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants